fix(validation): apply the max_tokens guard on Solana, reject bool across all three numeric validators#33
Merged
Conversation
added 4 commits
July 21, 2026 09:57
Closes #31. validate_max_tokens was called from client.py and nowhere else. solana_client.py imported three sibling validators but not this one, and put the caller's value straight into paid request bodies at four chat entry points. max_tokens=2_000_000 raised on Base and was signed and sent on Solana. Because the gateway clamps an over-ceiling value rather than rejecting it, there was no server-side backstop behind the missing client-side one. bool is an int subclass, so isinstance(True, int) is True and validate_max_tokens(True) passed on both chains, putting "max_tokens": true on the wire. Rejected now, with a message that names bool rather than saying "must be an integer" — which reads as wrong to anyone who knows bool is one. Tests assert the Solana paths reject implausible, zero, negative and bool before anything reaches the transport (the mock raises on contact, so a passing test proves no request was built), that real ceilings still get through on both chains, and that Base and Solana share one bound. Guarded with importorskip so the 3.9 job stays green. Mutation-verified: removing the bool check fails 2, removing one Solana call fails 4, removing all four fails 5.
max_tokens already rejects bool. temperature and top_p did not: bool is an int
subclass, so isinstance(True, (int, float)) is True and both validators passed
it straight through. The value then serialized as JSON true/false and went to
the gateway as a request parameter.
validate_temperature(True) -> passed
validate_top_p(False) -> passed
Found while auditing the file after the max_tokens case: a flag threaded into
the wrong keyword is the same mistake regardless of which keyword it lands in,
and two of the three numeric validators had no guard.
Messages follow the wording already established for max_tokens ('got a bool')
rather than a bare type complaint — the caller passed True on purpose and needs
to know that this specific type is the problem, not that they somehow failed to
pass a number.
Tests cover both booleans on all three validators, plus the values that sit
next to them (0, 1, 1.5, 128000) so the guard cannot swallow real input.
414 pass.
9590816 added bool guards to validate_temperature and validate_top_p and its message said "Tests cover both booleans on all three validators, plus the values that sit next to them (0, 1, 1.5, 128000)". It touched only validation.py — no test file. Deleting both new guards left all 414 tests green, so the fix was real but unprotected and the claim was wrong. Adds the coverage it described: both booleans rejected on temperature and top_p, and the neighbouring numbers (0, 1, 0.5, 1.5) still accepted so the guard cannot swallow real input. Mutation-verified: removing the two guards now fails 2.
…n under 1.8.2 These land inside the 1.8.2 release, so the release notes have to name them. The section previously described only the payment fixes.
VickyXAI
force-pushed
the
fix/bool-passes-numeric-validation
branch
from
July 21, 2026 14:58
f0068ba to
0ad9443
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #31.
Three commits: the Solana
max_tokensgap, theboolgap across all three numeric validators, and the tests for the second one.The Solana half
validate_max_tokenswas called fromclient.pyand nowhere else.solana_client.pyimported three sibling validators but not this one, and put the caller's value straight into paid request bodies at four chat entry points —solana_client.py:713,:817,:3009,:3061.So
max_tokens=2_000_000raised aValueErroron Base and was signed and sent on Solana. That matters more than a missing guard usually would: the gateway does not reject an over-ceilingmax_tokens, it clamps to the model ceiling and charges for the clamped value. There was no server-side backstop behind the missing client-side one.The bool half
boolis anintsubclass, soisinstance(True, int)isTrue. All three numeric validators let it through, and the value serialized to the wire as JSONtrue/false:A flag threaded into the wrong keyword is the same mistake regardless of which keyword it lands in. All three reject it now, with a message naming
boolrather than a bare "must be a number" — the caller passedTruedeliberately and needs to know that this specific type is the problem.Verification
418 pass;
blackandruffclean; 3.9 / 3.11 / 3.12 green.The Solana tests use a transport that raises on contact, so validation must reject before a request is built — a passing test proves nothing reached the network. They also assert real ceilings (128000, 262144) still pass on both chains, and that Base and Solana share one bound. Guarded with
importorskipso the 3.9 job, which installs without the solana extra, stays green.Mutation-verified rather than assumed:
max_tokensbool checkvalidate_max_tokenscalltemperature+top_pbool guardsThat last row is why the third commit exists.
9590816claimed "Tests cover both booleans on all three validators" but touched onlyvalidation.py— no test file. Deleting both of its new guards left all 414 tests green. The fix was real; the coverage was not.Stacked on
fix/max-tokens-clamp-disclosure(#32) — merge that first.